fix(browser): guard async doc open task against stale sheet pointer - #352
Merged
Conversation
Validate sheet uuid before delivering the queued DocOpenTask and re-resolve the renderer through the live sheet, instead of trusting the raw SheetRenderer* captured at queue time which may dangle once the sheet is destroyed or its address reused. 修复异步文档打开任务对悬空 sheet 指针的访问。交付排队任务前校验 sheet 的 uuid，并通过存活的 sheet 重新获取 renderer，不再使用排队时保存的裸 指针，避免 sheet 销毁或地址复用后调用 handleOpened 造成 use-after-free。 Log: 修复异步打开任务use-after-free Influence: 消除文档关闭后排队任务误判存活导致的崩溃与内存写坏，顺带修复任务丢弃路径的资源泄漏。
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning
详情 {
"export": {
"tests/test-prj-running.sh": {
"b": [
"export XDG_DATA_HOME=\"${build_path}/ut-testdata\""
]
}
}
} |
Reviewer's GuideThe PR prevents asynchronous document-open tasks from dereferencing stale sheet or renderer pointers by validating sheet identity at delivery time, re-resolving the live renderer, and cleaning up dropped payloads; it adds regression coverage and makes asynchronous tests and test execution more deterministic. Sequence diagram for guarded asynchronous document openingsequenceDiagram
participant SheetRenderer
participant PageRenderThread
participant DocSheet
participant Document
participant Pages
SheetRenderer->>DocSheet: uuid()
SheetRenderer->>PageRenderThread: appendTask(task)
PageRenderThread->>DocSheet: existSheet(sheet)
PageRenderThread->>DocSheet: uuid()
alt sheet is stale or uuid mismatches
PageRenderThread->>Pages: qDeleteAll(pages)
PageRenderThread->>Document: delete document
else sheet is valid
PageRenderThread->>DocSheet: renderer()
DocSheet-->>PageRenderThread: SheetRenderer
PageRenderThread->>SheetRenderer: handleOpened(error, document, pages)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
lzwind
approved these changes
Aug 31, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰。onDocOpenTask 方法重构逻辑正确：三重校验（null检查→存在性检查→UUID匹配），校验失败时正确清理资源（qDeleteAll(pages)、delete document）并返回，校验通过时通过存活sheet重新获取renderer调用handleOpened。SheetRenderer::openFileAsync 中正确设置task.uuid（带null检查）。DocSheet::uuid() 内联getter实现正确。测试脚本 is_djvu_exit_crash 函数逻辑正确，正确处理DjVu库退出阶段崩溃。边界条件处理完善：null sheet、地址复用、UUID不匹配均有覆盖。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 代码结构清晰，注释完整。每个变更点都有清晰的注释说明设计意图。建议：测试桩函数 renderer_stub() 中的堆分配对象可考虑使用智能指针管理生命周期，或在测试套件清理阶段统一释放。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。新增的null检查和UUID字符串比较开销可忽略不计。任务丢弃时的资源清理（qDeleteAll+delete）是必要操作，避免内存泄漏。UUID使用QString隐式共享，拷贝开销极低。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 存在0个安全漏洞。本次提交修复了use-after-free安全漏洞：旧代码使用排队时捕获的裸指针SheetRenderer*，在sheet销毁或地址复用后可能导致use-after-free。新代码通过UUID校验确保sheet存活，并通过存活sheet重新获取renderer，彻底消除了悬空指针风险。任务丢弃路径正确释放document和pages资源，修复了原有的内存泄漏。无新增安全漏洞。 💡 改进建议代码示例// 改进建议：测试桩中使用智能指针管理生命周期
#include <memory>
static SheetRenderer *renderer_stub()
{
// 使用静态 unique_ptr 管理生命周期，避免内存泄漏
static std::unique_ptr<SheetRenderer> dummy;
if (!dummy) {
dummy = std::make_unique<SheetRenderer>(nullptr);
}
return dummy.get();
}
// 或者在测试套件 TearDown 中统一清理
class TestPageRenderThread : public ::testing::Test {
protected:
void TearDown() override {
// 清理测试桩资源
cleanupStubs();
}
};本报告由 AI 代码审查工具自动生成 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Validate sheet uuid before delivering the queued DocOpenTask and re-resolve the renderer through the live sheet, instead of trusting the raw SheetRenderer* captured at queue time which may dangle once the sheet is destroyed or its address reused.
修复异步文档打开任务对悬空 sheet 指针的访问。交付排队任务前校验 sheet
的 uuid，并通过存活的 sheet 重新获取 renderer，不再使用排队时保存的裸
指针，避免 sheet 销毁或地址复用后调用 handleOpened 造成 use-after-free。
Log: 修复异步打开任务use-after-free
Influence: 消除文档关闭后排队任务误判存活导致的崩溃与内存写坏，顺带修复任务丢弃路径的资源泄漏。
Summary by Sourcery
Harden asynchronous document opening against stale sheet references and ensure discarded tasks are cleaned up safely.
Bug Fixes:
Enhancements:
CI:
Tests: